Skip to content

vendor: Bump library-go#710

Closed
wking wants to merge 4 commits intoopenshift:masterfrom
wking:bump-library-go
Closed

vendor: Bump library-go#710
wking wants to merge 4 commits intoopenshift:masterfrom
wking:bump-library-go

Conversation

@wking
Copy link
Copy Markdown
Member

@wking wking commented Dec 10, 2021

Generated with:

$ go get github.com/openshift/library-go
$ go mod tidy
$ go mod vendor
$ git add -A go.* vendor

using:

$ go version
go version go1.17.3 linux/amd64

I was originally planning on using this to pull in LeaderElectionSNOConfig from openshift/library-go#1242, but due to the size of the diff, I figured I might as well file the vendor bump as a stand-alone PR.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Dec 10, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 10, 2021
@wking
Copy link
Copy Markdown
Member Author

wking commented Dec 10, 2021

Hmm:

panic: cannot handle unexported field at {*payload.Update}.Manifests[2].id:

From our diff.ObjectReflectDiff call in pkg/payload/payload_test.go vs. this. I guess we should find another differ...

Generated with:

  $ go get github.com/openshift/library-go
  $ go mod tidy
  $ go mod vendor
  $ git add -A go.* vendor

using:

  $ go version
  go version go1.17.3 linux/amd64
@vrutkovs
Copy link
Copy Markdown

kube recommends using github.com/google/go-cmp/cmp.Diff instead

@damemi
Copy link
Copy Markdown

damemi commented Dec 22, 2021

Hi, just checking what is the status of this?

@vrutkovs
Copy link
Copy Markdown

vrutkovs commented Jan 3, 2022

imo this needs to be closed in favor of #716, which implements a new comparison

Because ObjectReflectDiff is deprecated and doesn't handle the
internal 'id' property.  We're using the recommended replacement [1].

[1]: https://github.com/kubernetes/apimachinery/blob/de7147de41c9331e4697399f48564f1d970cc01e/pkg/util/diff/diff.go#L70
Now that we're using these in the pkg/payload unit tests.  Generated
with:

  $ go mod tidy
  $ go mod vendor
  $ git add -A go.* vendor

using:

  $ go version
  go version go1.17.3 linux/amd64
@wking
Copy link
Copy Markdown
Member Author

wking commented Jan 4, 2022

I've just pushed b1a211a, bringing the github.com/google/go-cmp/cmp.Diff bit over from #716, but leaving the pkg/start pivot to pull lease defaults out for now. This PR is just the library-go vendor bump, and the minimal changes we need locally to work with that bump.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 4, 2022

@wking: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agnostic-upgrade b1a211a link true /test e2e-agnostic-upgrade
ci/prow/e2e-agnostic b1a211a link true /test e2e-agnostic

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@wking
Copy link
Copy Markdown
Member Author

wking commented Jan 7, 2022

#716 picked up my commits and builds on top, so no need for this separate PR.

/close

@openshift-ci openshift-ci Bot closed this Jan 7, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jan 7, 2022

@wking: Closed this PR.

Details

In response to this:

#716 picked up my commits and builds on top, so no need for this separate PR.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@wking wking deleted the bump-library-go branch January 7, 2022 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants